Skip to content

doc: writableFinished is true before 'finish'#28811

Closed
ronag wants to merge 1 commit intonodejs:masterfrom
nxtedition:fix-docs-writableFinished
Closed

doc: writableFinished is true before 'finish'#28811
ronag wants to merge 1 commit intonodejs:masterfrom
nxtedition:fix-docs-writableFinished

Conversation

@ronag
Copy link
Member

@ronag ronag commented Jul 23, 2019

The current description for stream.writableFinished is not entirely correct.

Checklist

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Jul 23, 2019
@Trott
Copy link
Member

Trott commented Jul 23, 2019

Practically, I"m not sure what the difference is to the reader? Is this about what finished is set to when your finish event listener runs or something like that?

@ronag ronag force-pushed the fix-docs-writableFinished branch from 459e4ad to d3b065b Compare July 23, 2019 05:29
@ronag
Copy link
Member Author

ronag commented Jul 23, 2019

It’s important in order to know whether when I register a finish listener if it will be called or not.

@ronag ronag force-pushed the fix-docs-writableFinished branch from d3b065b to 2502502 Compare July 23, 2019 07:53
@ronag ronag changed the title docs: writableFinished is true before 'finish' dos: writableFinished is true before 'finish' Jul 23, 2019
vsemozhetbyt
vsemozhetbyt reviewed Jul 23, 2019
View reviewed changes
@ronag ronag force-pushed the fix-docs-writableFinished branch from 2502502 to cc13c69 Compare July 23, 2019 10:16
sam-github
sam-github reviewed Jul 23, 2019
View reviewed changes
@lpinca lpinca changed the title dos: writableFinished is true before 'finish' doc: writableFinished is true before 'finish' Jul 23, 2019
@ronag ronag force-pushed the fix-docs-writableFinished branch 2 times, most recently from 5998562 to 0b677a6 Compare July 23, 2019 20:50
@Trott
Copy link
Member

Trott commented Jul 25, 2019

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3804/

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 25, 2019
@Trott
Copy link
Member

Trott commented Jul 26, 2019

@nodejs/streams

mcollina
mcollina requested changes Jul 26, 2019
View reviewed changes
@trivikr trivikr removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 26, 2019
@ronag ronag force-pushed the fix-docs-writableFinished branch from 0b677a6 to c3b6f5b Compare August 2, 2019 06:34
@ronag ronag force-pushed the fix-docs-writableFinished branch from c3b6f5b to 204b76b Compare August 2, 2019 06:35
@Trott
Copy link
Member

Trott commented Aug 2, 2019

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3819/

Trott
Trott reviewed Aug 2, 2019
View reviewed changes
mcollina
mcollina approved these changes Aug 2, 2019
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Trott pushed a commit to Trott/io.js that referenced this pull request Aug 2, 2019
@ronag @Trott
doc: writableFinished is true before 'finish'
0897782 
PR-URL: nodejs#28811
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@Trott
Copy link
Member

Trott commented Aug 2, 2019

Landed in 0897782

@Trott Trott closed this Aug 2, 2019
targos pushed a commit that referenced this pull request Aug 2, 2019
@ronag @targos
doc: writableFinished is true before 'finish'
0ac6d28 
PR-URL: #28811
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Aug 6, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Trott Trott Trott approved these changes

@mcollina mcollina mcollina approved these changes

+2 more reviewers

@sam-github sam-github sam-github left review comments

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@ronag @Trott @sam-github @mcollina @vsemozhetbyt @trivikr @nodejs-github-bot